-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement LWG-2381 Inconsistency in parsing floating point numbers #3364
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I see -- thanks for fixing these bugs in this (fairly crufty!) code 😸
Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
- eliminate some Yoda conditions - reduce the scope of `_Sticky` - compactify the use of `_Has_dec_leading_zero`
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: statementreply <statementreply@gmail.com>
Thanks again for this amazing work which I would not have been able to accomplish! 😻 I pushed minor nitpicky changes, nothing substantial (FYI @strega-nil-ms anyways). With the addition of the charconv test cases, and the "if it doesn't match, at least it should match The change is still higher risk than most changes, but I believe we're preserving ABI here, and this is a significant improvement over the status quo. Let's do it. |
This comment was marked as resolved.
This comment was marked as resolved.
I've had to push an additional commit to restore the dllexport surface.
|
@frederick-vs-ja The compiler back-end's internal test suite (specifically spec2k eon) found a bug in this PR:
#include <cstdio>
#include <sstream>
using namespace std;
int main() {
printf("_MSVC_STL_UPDATE: %d\n", static_cast<int>(_MSVC_STL_UPDATE));
istringstream iss{"0\n"};
float flt = 1729.0f;
iss >> flt;
if (iss) {
printf("flt: %.1000g\n", flt);
} else {
printf("Parsing failure!\n");
}
} Before this PR (VS 2022 17.6 Preview 5):
After this PR:
We definitely need test coverage for this scenario. |
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Thank you for this absolutely massive correctness fix and LWG issue resolution, which the maintainers would not have been able to accomplish! We really appreciate it! 😻 🎉 🚀 |
Fixes #2388. Fixes #1582. Fixes #3375. Fixes #3376. Fixes #3378.
Also works towards
And updates references to working draft in
<xlocnum>
to WG21-N4944.The new test file is derived from libc++'s test files, except that
"inf"
/"nan"
cases are skipped. It currently seems non-conforming to handle"inf"
/"nan"
withdo_get
.This PR also (possibly) fixes
twosix pre-existing bugs:std::hexfloat
should have no effect on parsing floating-point numbers."1.0e320"
fordouble
, infinity (or negative infinity) instead of 0.0 should be given.do_get
should not considerCHAR_MAX
or a non-positivechar
value ingrouping()
ends the grouping, since it means that the current group is unlimited, but the next group may still be limited."9.999 .... [1000 nines] ...9"
should be parsed as10
, not100
._Sticky
logic should also be applied to hexadecimal representations.